Map performance - use pattern match on size to reduce recursive function calls #1069
Conversation
|
Thanks for testing this out. The improvements seem decent but I've always found this undocumented special case strange and I'm not sure we want to proliferate this pattern. If we really want to avoid visiting foldMap :: Monoid m => (a -> m) -> Set a -> m
foldMap _ Tip = mempty
foldMap f (Set _ x l r) = go x l r
where
go x l r = case (l, r) of
(Bin _ lx ll lr, Bin _ rx rl rr) -> go lx ll lr <> f x <> go rx rl rr
(Bin _ lx ll lr, Tip) -> go lx ll lr <> f x
(Tip, Bin rx rl rr) -> f x <> go rx rl rr
(Tip, Tip) -> f xBut I haven't measured the effects of such a change. |
|
The version you posted is about 10 to 15% slower than what's there already. See this commit which adds the change it to It's worth asking why it's slower – it seems that pattern matching on the strict packed integer should be very fast, as there's no pointer chasing or indirection involved, pattern matching on the tips does require some indirection. From the comment in the link above, eliding 2/3 of the recursive calls to Tip is a big deal, because there are a lot of Tips, effectively about the same number as the number of items in the map. Maybe we should add a GHC style note about this optimisation. Edit: I've written one. |
HuwCampbell
force-pushed
the
master
branch
2 times, most recently
from
1dd98a1
to
d2093d3
Compare
This change incorporates the idea already used for
foldMapandtraverse, where wepattern match on the size of "leaf nodes". If the size is 1, we already know both sides
are tips, and we don't need to recurse.
Of particular interest is
mapMaybe, where we can also omit the extra function calls to link.Benchmarks show improvements around 10 to 20% for most operations affected,
though
foldl'seems closer to 30%.Some (very) cursory analysis says that about 1/3 of Bin nodes in a tree should have no
children, hence changes up to 30% do seem plausible.
Pattern matching on a strict and unboxed
Intshould be very fast.